Skip to content

Retain shared certificate and key buffers across CTX reloads - #12062

Open
ageprocpp wants to merge 4 commits into
wolfSSL:masterfrom
ageprocpp:fix/fenrir-16791
Open

Retain shared certificate and key buffers across CTX reloads#12062
ageprocpp wants to merge 4 commits into
wolfSSL:masterfrom
ageprocpp:fix/fenrir-16791

Conversation

@ageprocpp

@ageprocpp ageprocpp commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes Fenrir #16791

When certificate or key copying is disabled, a WOLFSSL object can share DER buffers with its WOLFSSL_CTX. Replacing a certificate chain or private key on the context after wolfSSL_new() can release a buffer that the existing connection still needs and cause use-after-free.

This change adds reference counting to DerBuffer so that a context and its connections can retain the same certificate, chain, or private key buffer and make the buffer freed only when its last reference is released.

Background

This revisits the reference-counting approach proposed in #11109, which superseded #10905 and was closed without being merged. The approach in #11191 instead blocks certain context changes during callbacks regarding handshake.

The case addressed here does not require a context change inside a callback: the context can be always reloaded after wolfSSL_new() and before the connection uses its certificate or key and that is the condition of the case.

Copilot AI balanced review requested due to automatic review settings November 9, 2026 06:38
Copilot AI reviewed Nov 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions

github-actions Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

gcc-arm-cortex-m0plus

  • FLASH: .text +88 B (+0.1%, 70,891 B / 262,144 B, total: 27% used)

gcc-arm-cortex-m3

  • FLASH: .text +412 B (+0.3%, 132,173 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m4

  • FLASH: .text +448 B (+0.2%, 209,484 B / 262,144 B, total: 80% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text +64 B (+0.1%, 73,591 B / 262,144 B, total: 28% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text +64 B (+0.0%, 181,779 B / 262,144 B, total: 69% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +384 B (+0.2%, 196,376 B / 1,048,576 B, total: 19% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text +64 B (+0.1%, 68,185 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text +832 B (+0.1%, 802,020 B / 1,048,576 B, total: 76% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text +64 B (+0.0%, 222,932 B / 262,144 B, total: 85% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +384 B (+0.1%, 313,232 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +384 B (+0.1%, 344,080 B / 1,048,576 B, total: 33% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text +64 B (+0.1%, 68,185 B / 262,144 B, total: 26% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text +384 B (+0.3%, 132,945 B / 262,144 B, total: 51% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text +384 B (+0.2%, 250,018 B / 262,144 B, total: 95% used)

gcc-arm-cortex-m7

  • FLASH: .text +448 B (+0.2%, 209,484 B / 262,144 B, total: 80% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +384 B (+0.1%, 314,064 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +384 B (+0.2%, 250,018 B / 262,144 B, total: 95% used)

No memory changes detected for:

Catch memory regressions in CI with MemBrowse. Free for open source.

@ageprocpp
ageprocpp force-pushed the fix/fenrir-16791 branch 3 times, most recently from e069aa8 to 142c6f6 Compare November 9, 2026 08:02
@ageprocpp ageprocpp assigned ageprocpp and wolfSSL-Bot and unassigned ageprocpp Nov 9, 2026
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot previously requested changes Nov 9, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #12062

Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfcrypt-port-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): doc/dox_comments/header_files/asn_public.h
Coverage: 10 of 12 in-scope changed file(s) opened by the reviewer; not opened: wolfssl/internal.h, wolfssl/wolfcrypt/asn.h

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Max

@ageprocpp ageprocpp assigned ageprocpp and unassigned wolfSSL-Bot Nov 9, 2026
wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Nov 10, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #12062

Scan targets checked: wolfssl-src, wolfssl-bugs
Unchanged since last review (not re-run): wolfcrypt-src, wolfcrypt-bugs, wolfcrypt-port-bugs
Not reviewed (no scan target covers them): doc/dox_comments/header_files/asn_public.h
Coverage: 2 of 3 in-scope changed file(s) opened by the reviewer; not opened: tests/api.c

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Review tier: Lite

@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot dismissed their stale review November 10, 2026 08:19

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

@ageprocpp ageprocpp assigned wolfSSL-Bot and unassigned ageprocpp Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@wolfSSL-Bot wolfSSL-Bot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ageprocpp @wolfSSL-Fenrir-bot @wolfSSL-Bot